Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use builtin:: functions where available #132

Merged
merged 1 commit into from
Aug 2, 2024
Merged

Conversation

leonerd
Copy link
Contributor

@leonerd leonerd commented Jun 12, 2024

On Perl 5.40 and above, use the various builtin:: functions to provide those in Scalar::Util, avoiding the need for our own custom xsubs in that case

…e those in Scalar::Util, avoiding the need for our own custom xsubs in that case
@karenetheridge
Copy link
Member

Some of these functions were available in 5.38's builtin, so should we start with that version?

@leonerd
Copy link
Contributor Author

leonerd commented Jun 13, 2024

Some of these functions were available in 5.38's builtin, so should we start with that version?

They're present in 5.38 (or even 5.36) but in those versions they print experimental warnings. It isn't until 5.40 when they become stable.

@karenetheridge
Copy link
Member

karenetheridge commented Jun 13, 2024

Did they change between 5.38 and 5.40? If they didn't, we could retroactively consider them stable in 5.38, in the same way that the stable declaration does in experimental.pm. (Therefore, for 5.38, the Scalar::Util shim would need to silence the experimental warning.)

@haarg
Copy link
Member

haarg commented Jun 13, 2024

The functions would need a wrapper sub to be able to silence the warning.

@leonerd
Copy link
Contributor Author

leonerd commented Jun 14, 2024

Yeah, there's only one warning category, not one per function, so it's not possible to silence just the now-stable functions. I don't see it as a huge problem though - the XS versions will have to live here until we decide to drop support for anything older than 5.40 and I don't imagine that happening for a decade or two at least ;) Whether the horizon is 5.38 or 5.40 is no huge problem between those.

@leonerd
Copy link
Contributor Author

leonerd commented Jul 26, 2024

There doesn't seem to be any overriding reason or argument not to do this, so if no objections by Aug 1st I shall merge it.

@leonerd leonerd merged commit 72720f1 into master Aug 2, 2024
40 checks passed
@leonerd leonerd deleted the use-builtin-on-5.40 branch August 6, 2024 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants